Skip to content

fix: Assorted clang-tidy fixes - #7881

Open
mvadari wants to merge 4 commits into
developfrom
mvadari/ctidy-07-26
Open

fix: Assorted clang-tidy fixes#7881
mvadari wants to merge 4 commits into
developfrom
mvadari/ctidy-07-26

Conversation

@mvadari

@mvadari mvadari commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Title pretty much says it all - these changes came out of triggering clang-tidy and fixing build issues.

Context of Change

clang-tidy triggered these changes on a separate PR, I moved them here since these changes were wholly unrelated to my PR

API Impact

N/A

@mvadari
mvadari requested review from godexsoft and mathbunnyru July 27, 2026 21:39

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical relay and dedup bugs—see inline.

Comment thread include/xrpl/core/HashRouter.h Outdated
return false;
relayed_.emplace(now);
return true;
return !relayed_ && *relayed_ + relayTime > now;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undefined behavior + missing state update breaks relay suppression:

Suggested change
return !relayed_ && *relayed_ + relayTime > now;
if (relayed_ && *relayed_ + relayTime > now)
return false;
relayed_.emplace(now);
return true;

return true;
return !processed_ && ((*processed_ + interval) > now);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undefined behavior + missing state update breaks dedup processing:

Suggested change
if (processed_ && ((*processed_ + interval) > now))
return false;
processed_.emplace(now);
return true;

Comment thread include/xrpl/core/HashRouter.h Outdated
Comment thread include/xrpl/core/HashRouter.h Outdated

@mathbunnyru mathbunnyru left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is likely to be a result of misconfiguration of your system-installed tools or a conflict with tolling provided by nix. For example, if you use nix-provided compiler (in nix devshell), you should also use nix-provided clang-tidy tooling.

There was also a bug, where a versioned tooling was not provided in nix environment, fixed already (#7844)

Please, try to reproduce in latest develop, and if you still see the diff, please, provide the reproducible example how you run stuff.

@mathbunnyru mathbunnyru left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, changes themselves look fine.
I would still check the reason why it happens on your machine though - to reduce chances of unrelated changes in the future

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants